feat(catalog): enable logo file upload in theme editor by drernie · Pull Request #4781 · quiltdata/quilt

drernie · 2026-03-24T18:35:43Z

Summary

Implements useUploadFile in CatalogSettings.tsx to upload the logo image to the service bucket at catalog/logo.<ext> and return an s3:// URL
Removes the useThirdPartyDomainForLogo flag in ThemeEditor.tsx so the drag-and-drop file input is shown instead of the URL text field
The Logo component already supports s3:// URLs (signs them via AWS credentials before rendering), so no further changes are needed

Test plan

In Admin > Settings > Theme, click "Configure theme" — verify a drag-and-drop zone appears instead of a URL text field
Drop a PNG/JPG logo image — verify it uploads to s3://<service-bucket>/catalog/logo.<ext> and the logo appears in the navbar
Verify that removing the theme also clears the logo
All existing Logo unit tests pass (vitest run "Logo")

🤖 Generated with Claude Code

Greptile Summary

This PR activates logo file upload in the theme editor by implementing useUploadFile in CatalogSettings.tsx (uploading to catalog/logo.<ext> in the service S3 bucket) and flipping useThirdPartyDomainForLogo to false in ThemeEditor.tsx so the drag-and-drop InputFile component is shown instead of the URL text field.

Key points:

The useUploadFile implementation is clean and correct — reads the file buffer, infers the extension from the filename, uploads via s3.putObject, and returns an s3:// URL that the existing Logo component can sign and render.
Enabling InputFile exposes a pre-existing URL.createObjectURL memory leak in that component: blob URLs are created on every file drop but URL.revokeObjectURL is never called — neither on value change nor on unmount. This code path was unreachable before this PR.
The dropzone has no accept constraint, allowing non-image files to be uploaded to S3 as a logo.
useThirdPartyDomainForLogo is now a dead constant (false); the ternary branch for true in the render is unreachable and can be removed.

Confidence Score: 4/5

Safe to merge after fixing the ObjectURL memory leak; the remaining issues are non-blocking style/best-practice cleanups.
The core upload logic is correct and well-scoped. The memory leak in InputFile is a real bug activated by this PR (P1) but it only affects the admin settings dialog — a low-traffic, admin-only surface — so it won't cause production reliability problems in normal usage. The missing accept restriction and the dead code are P2 cleanups.
catalog/app/containers/Admin/Settings/ThemeEditor.tsx — the InputFile component's URL.createObjectURL call needs a corresponding revokeObjectURL cleanup.

Important Files Changed

Filename	Overview
catalog/app/utils/CatalogSettings.tsx	Implements `useUploadFile` — uploads a file to `catalog/logo.<ext>` in the service S3 bucket and returns an `s3://` URL. Implementation is straightforward; minor note that old logo files at different extensions (e.g. `logo.png` vs `logo.jpg`) are never cleaned up on overwrite.
catalog/app/containers/Admin/Settings/ThemeEditor.tsx	Flips `useThirdPartyDomainForLogo` to `false`, activating the `InputFile` dropzone branch. This exposes a pre-existing `URL.createObjectURL` memory leak (no `revokeObjectURL` cleanup) and a missing `accept` constraint on the dropzone. The flag variable itself becomes dead code.

Sequence Diagram

sequenceDiagram
    actor User
    participant InputFile
    participant ThemeEditor
    participant useUploadFile
    participant S3

    User->>InputFile: Drop image file
    InputFile->>ThemeEditor: onChange(File)
    User->>ThemeEditor: Click Save
    ThemeEditor->>useUploadFile: uploadFile(File)
    useUploadFile->>S3: putObject to catalog/logo.ext
    S3-->>useUploadFile: success
    useUploadFile-->>ThemeEditor: object URL returned
    ThemeEditor->>S3: writeSettings with updated logo
    S3-->>ThemeEditor: success
    ThemeEditor->>ThemeEditor: close dialog

Comments Outside Diff (2)

catalog/app/containers/Admin/Settings/ThemeEditor.tsx, line 132-135 (link)

ObjectURL memory leak — revocation missing

URL.createObjectURL creates a persistent blob URL that is never released. Every time the user drops a new file, the old blob URL is abandoned in memory and never cleaned up. When the dialog closes and the component unmounts, the same happens. This branch was dead code before this PR (guarded by the now-removed useThirdPartyDomainForLogo = true flag), so this leak is effectively introduced here.

A useEffect with cleanup should replace the useMemo:
catalog/app/containers/Admin/Settings/ThemeEditor.tsx, line 128-131 (link)

Dropzone accepts all file types

The dropzone has no accept constraint, so users can drop arbitrary files (PDFs, videos, executables, etc.) as a logo. These will be uploaded to S3 and saved as the logo URL. Adding an accept list restricts both the native file picker and the drop validation:

_{Reviews (1): Last reviewed commit: "feat(catalog): enable logo file upload i..." | Re-trigger Greptile}

Greptile also left 1 inline comment on this PR.

Implement useUploadFile to upload the logo image to the service bucket at catalog/logo.<ext> and return an s3:// URL, then wire it up by removing the useThirdPartyDomainForLogo flag so the drag-and-drop InputFile component is shown instead of the URL text field. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

claude

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review.

_{Tip: disable this comment in your organization's Code Review settings.}

codecov · 2026-03-24T18:37:06Z

Codecov Report

❌ Patch coverage is 0% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 44.31%. Comparing base (7077590) to head (68467b0).

Files with missing lines	Patch %	Lines
catalog/app/utils/CatalogSettings.tsx	0.00%	7 Missing and 2 partials ⚠️
...alog/app/containers/Admin/Settings/ThemeEditor.tsx	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4781      +/-   ##
==========================================
- Coverage   44.32%   44.31%   -0.01%     
==========================================
  Files         813      813              
  Lines       32736    32742       +6     
  Branches     5721     5724       +3     
==========================================
  Hits        14511    14511              
- Misses      16224    16228       +4     
- Partials     2001     2003       +2

Flag	Coverage Δ
api-python	`92.65% <ø> (ø)`
catalog	`19.52% <0.00%> (-0.01%)`	⬇️
lambda	`96.63% <ø> (ø)`
py-shared	`98.18% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

greptile-apps · 2026-03-24T18:38:37Z

catalog/app/containers/Admin/Settings/ThemeEditor.tsx


-  // FIXME: remove when file upload would be ready
-  const useThirdPartyDomainForLogo = true
+  const useThirdPartyDomainForLogo = false


Dead code — constant always false

useThirdPartyDomainForLogo is now permanently false, making the useThirdPartyDomainForLogo ? <URLField> : <InputFile> ternary at line 358 unreachable on the true branch. The whole conditional and the variable can be removed, leaving only the <InputFile> field. This keeps the file clean and avoids confusion for future readers.

Suggested change

const useThirdPartyDomainForLogo = false

(Remove this line and inline the <InputFile> field directly, deleting the ternary around it.)

drernie · 2026-03-25T04:21:35Z

Test results against quilt-staging (us-east-1)

All 9/9 checks passed after running the E2E test suite with a headed Playwright browser against the live nightly stack.

Results

Check	Result
Catalog loads authenticated	✅
Admin Settings page loads with Theme section	✅
Drop-zone InputFile rendered (not URL text field)	✅
URL text field absent	✅
Logo preview visible after file selection	✅
Dialog closed after Save	✅
S3 PUT to `catalog/logo.*` captured	✅
Logo image visible in navbar	✅
`settings.json` persisted with `s3://…/catalog/logo.png`	✅

Infrastructure change required

The catalog-config inline policy on the ReadWriteQuiltV2 IAM role only allowed s3:PutObject on catalog/settings.json. The logo upload writes to catalog/logo.<ext>, which was blocked (403). The policy needs one additional statement in the stack template that provisions this role:

{
  "Action": "s3:PutObject",
  "Resource": "arn:aws:s3:::${ServiceBucket}/catalog/logo.*",
  "Effect": "Allow"
}

Applied to staging manually for testing; needs to be added to the stack CDK/CFN definition before deploying to other stacks.

claude bot reviewed Mar 24, 2026

View reviewed changes

greptile-apps bot reviewed Mar 24, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(catalog): enable logo file upload in theme editor#4781

feat(catalog): enable logo file upload in theme editor#4781
drernie wants to merge 1 commit intomasterfrom
feat/logo-file-upload

drernie commented Mar 24, 2026 •

edited by greptile-apps bot

Loading

Uh oh!

claude bot left a comment

Uh oh!

codecov bot commented Mar 24, 2026 •

edited

Loading

Uh oh!

greptile-apps bot Mar 24, 2026

Uh oh!

drernie commented Mar 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

drernie commented Mar 24, 2026 • edited by greptile-apps bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Sequence Diagram

Comments Outside Diff (2)

Uh oh!

claude bot left a comment

Choose a reason for hiding this comment

Claude Code Review

Uh oh!

codecov bot commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

greptile-apps bot Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

drernie commented Mar 25, 2026

Test results against quilt-staging (us-east-1)

Results

Infrastructure change required

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

drernie commented Mar 24, 2026 •

edited by greptile-apps bot

Loading

codecov bot commented Mar 24, 2026 •

edited

Loading